-
Notifications
You must be signed in to change notification settings - Fork 497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dbt v1 compat #453
dbt v1 compat #453
Conversation
* Update require-dbt-version to be 1.0 * Fix SQL 42000 on Exasol (#420) " SQL-Error [42000]: syntax error, unexpected '*' " If you specify the * in the unioned with their respectiv names <name>.* you do not receive the SQL Error posted above. This should not inflict any further problems since it is redundant for most DBs. * Minor readme link fixes (#431) * minor readme link fixes * changelog addition Co-authored-by: Joel Labes <[email protected]> * 0.7.4 changelog (#432) * Update CHANGELOG.md * Note branch name change * use `limit_zero` macro instead of `limit 0` (#437) * Utils 0.7.4b1 (#433) * Update require-dbt-version to be 1.0 * Fix SQL 42000 on Exasol (#420) " SQL-Error [42000]: syntax error, unexpected '*' " If you specify the * in the unioned with their respectiv names <name>.* you do not receive the SQL Error posted above. This should not inflict any further problems since it is redundant for most DBs. * Minor readme link fixes (#431) * minor readme link fixes * changelog addition Co-authored-by: Joel Labes <[email protected]> * 0.7.4 changelog (#432) * Update CHANGELOG.md * Note branch name change Co-authored-by: Timo Kruth <[email protected]> Co-authored-by: Joe Markiewicz <[email protected]> * standard convention * Update integration_tests/tests/jinja_helpers/test_slugify.sql Taking the liberty of committing on your behalf so that the CI job starts again * Change limit_zero to be a macro Co-authored-by: Joel Labes <[email protected]> Co-authored-by: Timo Kruth <[email protected]> Co-authored-by: Joe Markiewicz <[email protected]> * Add col_name alias to else state too (#437) * Remove extra semicolon in `insert_by_period` materialization (#439) * Remove extra semicolon in `insert_by_period` materialization. `create_table_as()` generates a SQL statement that already ends with a semicolon, so the extra semicolon after a `create_table_as()` call in the `insert_by_period` materialization ends up being an empty SQL statement, and at least when using Snowflake this causes the dbt run to fail with a "cannot unpack non-iterable NoneType object" error. * Update changelog for PR 439. * Use the relation object passed into get_column_values, instead of making our own (#440) * Use the relation object passed into get_column_values, instead of making our own * Rename variables in get column value test to be clearer * Update CHANGELOG.md * Update CHANGELOG.md Co-authored-by: Timo Kruth <[email protected]> Co-authored-by: Joe Markiewicz <[email protected]> Co-authored-by: Anders <[email protected]> Co-authored-by: Sean Rose <[email protected]>
…448) * Create integration test for a dropped relation * Update get_column_values.sql * Swap out adapter call for a good old fashioned drop table
@jtcohen6 beyond the code review, I'm not sure where the config is for the version of dbt used in integration tests - can you give me a pointer? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already installing the latest compatible prerelease version of dbt-<adapter>
:
Line 9 in db8a047
pip install --pre "dbt-$1" |
The problem is, our Circle containers are running on python 3.6, and dbt-core==1.0.0rc
dropped support for python 3.6 (it's reaching end of life on Dec 23). So I think you'll just want to upgrade these container versions from 3.6.13
to something newer, e.g. cimg/python:3.9.9
(docs):
dbt-utils/.circleci/config.yml
Line 8 in db8a047
- image: circleci/python:3.6.13-stretch |
integration_tests/dbt_project.yml
Outdated
data_get_column_values_dropped: | ||
# TODO: Ideally this would use the adapter, but it gives a "Tried to drop relation, but its type is null" error. | ||
#+post-hook: "{% do adapter.drop_relation(this) %}" | ||
+post-hook: "drop table {{ this }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you know that this is a table, you could try something like (pseudo-code):
+post-hook: "{% do adapter.drop_relation(this.incorporate(type='table')) %}"
dbt_project.yml
Outdated
|
||
require-dbt-version: [">=0.20.0", "<=1.0.0"] | ||
require-dbt-version: [">=1.0.0-rc", "<2.0.0"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about switching this to [">=1.0.0", "<2.0.0"]
once we have a final release of dbt-core v1.0.0? In which case, this feels fine for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I don't really want to have to release 0.8.1 in a week that just drops the release candidates. Since
- 0.7.5 supports <= 1.0 as its upper bound, which does include the RCs, and
- 1.0 final is on the cusp of release anyway,
I think I'm happy to say that 0.8 is only going to cover the final released version of 1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With that said, that means that when I merge this in, Circle CI will give it a ❌ until 1.0 final actually ships. Do we feel OK about that? (cc @jasnonaz)
keeping in mind that there are like a dozen packages that will all need changed, so I really don't wanna do that twice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.0!!!
Approving this with failing tests as the tests are failing due to the fact that this version of utils will not be compatible with release candidates of dbt v1.0.0, only with the final version. We have a successful CI run here. The only change made subsequent to this run was bumping the bound to be >1.0.0, which, as expected, caused the tests to fail.
This is a:
master
dev/
branchdev/
branchDescription & motivation
🥳 ONE POINT OH IS COMING BABYYYYYYYYYY
🧐 New minor version which supports any 1.x.x version of Core.
I'm not pleased that I have to do
require-dbt-version: [">=1.0.0-rc", "<2.0.0"]
. My original attempt wasrequire-dbt-version: "~=1.0.0"
and that didn't work:Nor did
require-dbt-version: [">=1.0.0", "<2.0.0"]
because it didn't pick up the release candidates.Open to a different approach!
Checklist
star()
source)limit_zero()
macro in place of the literal string:limit 0
dbt_utils.type_*
macros instead of explicit datatypes (e.g.dbt_utils.type_timestamp()
instead ofTIMESTAMP